Skip to content

Conversation

ShahazadAbdulla
Copy link

@ShahazadAbdulla ShahazadAbdulla commented Sep 2, 2025

Fixes #2166

This PR adds the update_rate and is_async parameters to the RQT controller manager details popup window.

Implementation Details:

  • Modified popup_info.ui to add new QLabel widgets for displaying the information.
  • Updated the _on_ctrl_info function in controller_manager.py to fetch the parameters using ros2param.api.call_get_parameters.
  • The logic first attempts to retrieve per-controller parameters (<controller_name>.update_rate, etc.).
  • If a per-controller value is not found, it gracefully falls back to the global update_rate or a sensible default (False for is_async).

Testing:

  • The feature was developed and tested in a ROS 2 Rolling Docker environment.
  • When running the rrbot demo from ros2_control_demos, the popup correctly displays the global and default values as shown below.

Result:

screenshot-20250902_135959 screenshot-20250902_140045

Adds update_rate and is_async parameters to the RQT controller manager details popup. Fetches per-controller parameters and falls back to global/default values if not available. Fixes ros-controls#2166.
@bmagyar
Copy link
Member

bmagyar commented Sep 3, 2025

@ShahazadAbdulla could you please reattempt uploading the screenshot?

Copy link

codecov bot commented Sep 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.27%. Comparing base (31c4bb1) to head (ee2dcc5).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2510   +/-   ##
=======================================
  Coverage   89.27%   89.27%           
=======================================
  Files         145      145           
  Lines       16484    16484           
  Branches     1395     1395           
=======================================
  Hits        14716    14716           
  Misses       1229     1229           
  Partials      539      539           
Flag Coverage Δ
unittests 89.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ShahazadAbdulla
Copy link
Author

ShahazadAbdulla commented Sep 3, 2025

@ShahazadAbdulla could you please reattempt uploading the screenshot?

ofc. @bmagyar

screenshot-20250902_135959 screenshot-20250902_140045

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

I tested it, and it works for controllers, but: The same layout is used for controllers and hardware components, but you only query the parameters from the controllers. If you later open the details of a hardware component, the data is wrong because not updated/deleted any more.

Please consider a consistent code style (comments), and remove LLM output

Comment on lines +323 to +327
# Update the labels you created in the .ui file
popup.label_update_rate_value.setText(update_rate)
popup.label_async_setting_value.setText(is_async)


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds like some LLM description ;)

Suggested change
# Update the labels you created in the .ui file
popup.label_update_rate_value.setText(update_rate)
popup.label_async_setting_value.setText(is_async)
popup.label_update_rate_value.setText(update_rate)
popup.label_async_setting_value.setText(is_async)

node=self._node, node_name=self._cm_name, parameter_names=param_names)

# Check the response for valid types
# In Rolling, INTEGER is type 2, BOOL is type 1, NOT_SET is 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# In Rolling, INTEGER is type 2, BOOL is type 1, NOT_SET is 0
# INTEGER is type 2, BOOL is type 1, NOT_SET is 0

popup.ctrl_type.setText(ctrl.type)


#Fetch the update_rate and is_async
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#Fetch the update_rate and is_async
# Fetch the update_rate and is_async

if response.values[1].type == 1: # Found per-controller is_async
is_async = str(response.values[1].bool_value)

# --- If per-controller update_rate not found, check global ---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# --- If per-controller update_rate not found, check global ---
# If per-controller update_rate not found, check global

if response_global.values and response_global.values[0].type == 2:
update_rate = f"{response_global.values[0].integer_value} Hz (Global)"

# --- Set a default for is_async if not found ---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# --- Set a default for is_async if not found ---
# Set a default for is_async if not found

is_async = "False (Default)"

except Exception as e:
# Use the correct '.warning()' method and no exc_info
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Use the correct '.warning()' method and no exc_info

?

is_async = "N/A" # Default to N/A for now

try:
# --- Check for per-controller params first ---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# --- Check for per-controller params first ---
# Check for per-controller params first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rqt_controller_manager: Add more information to details-window
3 participants